Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[full-ci] feat(public-links): archive downloads in public links #5924

Merged
merged 17 commits into from
Nov 8, 2021

Conversation

fschade
Copy link
Contributor

@fschade fschade commented Oct 18, 2021

Description

We introduced archive downloads and app providers in the last weeks, in perspective of getting this out fast we decided to ship those features in small units.

Currently it is not possible to retrieve capabilities from public links which is problematic and prevents usage of features like archiving and app providers.

This is included and now loads the capabilities in public links with password protection and without.
After that was done it was needed to update the launch process of web which now also have a new livecycle hook (userReady).

This also works if the runtime user provisioning step is skipped and another application takes care, in our case the files app.
This shifts responsibility to the individual app and for now is a highly experimental api which should not be used outside of this scope.

some hacky parts are included which needs a refactoring & new design of how applications get loaded, provisioned and rendered. Different topic.

Following features benefit from it:

  • create archive from an public link
  • create archive from an password protected public link (currently disabled because ocis implications)
  • open a file with an app provider from an public link
  • open a file with an app provider from an password protected public link
  • retrieving capabilities for public links

it also fixes a bug where the capabilities where not available right after a fresh login, we had to do a hard reload before this to work around it.

Related Issue

Motivation and Context

be able to use new features for public links also

How Has This Been Tested?

  • local ocis installation

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • write tests
  • test more
  • changelog, TODO is probbally not the best idea ;)

@update-docs
Copy link

update-docs bot commented Oct 18, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/19809/11/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@fschade fschade force-pushed the public-token-features branch 2 times, most recently from 056cd7b to 51f246a Compare October 21, 2021 18:38
@ownclouders
Copy link
Contributor

Results for oCISBasic https://drone.owncloud.com/owncloud/web/19864/49/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@labkode
Copy link

labkode commented Oct 22, 2021

  • create archive from an password protected public link (currently disabled because ocis implications)

@fschade can you give a pointer to this limitation?

@wkloucek
Copy link
Contributor

I did not succeed in the bookmark usecase for the app provider:

@wkloucek
Copy link
Contributor

  • open a file with an app provider from an password protected public link

Is this already supposed to work?
I didn't succeed with this. I get a basic auth popup and if i cancel that, I don't have the apps in the context menues.

Peek.2021-10-22.08-37.mp4

@wkloucek
Copy link
Contributor

Nothing related to Web, but I am currently able to edit and save changes to files I received with a "viewer" public link. This needs to be fixed Reva.

@wkloucek
Copy link
Contributor

Also the public token auth impersonates the original sharer and therefore the apps show the original sharer as user. It should be highlighted that it isn't the original sharer but an anonymous user granted by the original sharer.

image

@labkode
Copy link

labkode commented Oct 22, 2021

Nothing related to Web, but I am currently able to edit and save changes to files I received with a "viewer" public link. This needs to be fixed Reva.

@C0rby can you look into this one?

@fschade
Copy link
Contributor Author

fschade commented Oct 22, 2021

  • open a file with an app provider from an password protected public link

Is this already supposed to work? I didn't succeed with this. I get a basic auth popup and if i cancel that, I don't have the apps in the context menues.

Peek.2021-10-22.08-37.mp4

good point, i only tested it to open the link after files app showed up at least once, this is a bug. Thanks

@fschade
Copy link
Contributor Author

fschade commented Oct 22, 2021

@wkloucek the bookmark thing should work now.
@labkode we are at the moment not able to use the archiver from a public link, we have no way for now to pass the publikPassword to the backend.

Same could apply for appProviders if the link is bookmarked and the user haven't already provided his password

@CLAassistant
Copy link

CLAassistant commented Nov 4, 2021

CLA assistant check
All committers have signed the CLA.

@fschade fschade force-pushed the public-token-features branch from 23c2b50 to d7839d3 Compare November 4, 2021 14:04
@ownclouders
Copy link
Contributor

Results for oCISSharingInternalUsers2 https://drone.owncloud.com/owncloud/web/20042/58/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISSharingInternalUsers2 https://drone.owncloud.com/owncloud/web/20048/58/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISFiles2 https://drone.owncloud.com/owncloud/web/20049/55/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@kulmann kulmann changed the title [WIP] feat(public-links): archive downloads in public links [full-ci] feat(public-links): archive downloads in public links Nov 5, 2021
@kulmann kulmann marked this pull request as ready for review November 5, 2021 11:37
@kulmann kulmann force-pushed the public-token-features branch from d7839d3 to 4408b47 Compare November 5, 2021 11:44
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, hacky parts are ok for now

fschade and others added 4 commits November 5, 2021 14:17
update the launch process to support late loaded data like capabilities which is needed for doing things like archiving on public links
@kulmann kulmann force-pushed the public-token-features branch from da8e39c to 6ced5fc Compare November 5, 2021 15:27
@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/20109/15/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:15

@fschade fschade force-pushed the public-token-features branch from 3ebe54c to efd445b Compare November 8, 2021 10:24
@wkloucek
Copy link
Contributor

wkloucek commented Nov 8, 2021

Nothing related to Web, but I am currently able to edit and save changes to files I received with a "viewer" public link. This needs to be fixed Reva.

@C0rby can you look into this one?

Will be fixed in cs3org/reva#2214

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 8, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

10.4% 10.4% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10Sharing1 https://drone.owncloud.com/owncloud/web/20122/19/1
The following scenarios passed on retry:

  • webUISharingExpirationDate/shareWithExpirationDate.feature:14

@fschade fschade merged commit 936a57b into master Nov 8, 2021
@delete-merged-branch delete-merged-branch bot deleted the public-token-features branch November 8, 2021 13:29
@diocas
Copy link
Contributor

diocas commented Nov 9, 2021

With this PR merges, does that mean that all the others (#5863 #2479) can be closed?
Does this require anything special from the backend? What I'm seeing from a quick test, is a call to capabilities without any token (which in our case falls back to basic auth). I cancel the auth and the page loads, but then I don't see any call to the apps provider, hence no apps.
(using my text-editor app it redirects me to the idp, I guess I need to set the route as non authenticated? I already have a special route for public-links)

@wkloucek
Copy link
Contributor

wkloucek commented Nov 9, 2021

With this PR merges, does that mean that all the others (#5863 #2479) can be closed? Does this require anything special from the backend? What I'm seeing from a quick test, is a call to capabilities without any token (which in our case falls back to basic auth). I cancel the auth and the page loads, but then I don't see any call to the apps provider, hence no apps. (using my text-editor app it redirects me to the idp, I guess I need to set the route as non authenticated? I already have a special route for public-links)

cs3org/reva#2143 and owncloud/ocis#2536 needs to be there. The authentication for the capabilities request is done with the public-token header. A curl example would be curl 'https://ocis.ocis-traefik.latest.owncloud.works/ocs/v1.php/cloud/capabilities?format=json' -H 'PUBLIC-TOKEN:KvhXJieGpENGXgQ' but KvhXJieGpENGXgQ needs to be a valid / existing public link token

@diocas
Copy link
Contributor

diocas commented Nov 9, 2021

@ishank011

@wkloucek
Copy link
Contributor

@ishank011 please consider using it in combination with cs3org/reva#2214 only

@ishank011
Copy link

Thanks @wkloucek! I'll review that PR asap

@fschade
Copy link
Contributor Author

fschade commented Nov 10, 2021

@diocas please note this only works for public links without password, #5863 is used as tracking for public links with password

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
10 participants